Fix adjoint sensitivities for events that are both state-triggered and state-updating - #3258
Open
dweindl wants to merge 5 commits into
Open
Fix adjoint sensitivities for events that are both state-triggered and state-updating#3258dweindl wants to merge 5 commits into
dweindl wants to merge 5 commits into
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #3258 +/- ##
==========================================
- Coverage 78.52% 77.95% -0.57%
==========================================
Files 318 318
Lines 22197 22203 +6
Branches 1491 1491
==========================================
- Hits 17430 17309 -121
- Misses 4759 4886 +127
Partials 8 8
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
dweindl
force-pushed
the
fix-asa-event-trigger-time-sensitivity
branch
2 times, most recently
from
September 9, 2026 20:28
583959f to
8a7efac
Compare
dweindl
force-pushed
the
fix-asa-event-trigger-time-sensitivity
branch
from
September 10, 2026 07:23
8a7efac to
67589de
Compare
dweindl
commented
Sep 10, 2026
| "01104": True, | ||
| "01107": True, | ||
| "01148": True, | ||
| # Adjoint sensitivities & events. To revisit later. |
dweindl
marked this pull request as ready for review
September 10, 2026 11:39
FFroehlich
approved these changes
Sep 10, 2026
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Sep 10, 2026
…d state-updating deltaxB/deltaqB embed dtaudx/dtaudp (trigger-time-sensitivity terms), but these were evaluated at whatever state the generated function happens to be called with -- in practice the post-event state. That's the wrong side of the discontinuity: the equivalent forward-mode quantity (stau) is deliberately computed pre-event. Fix: eliminate w via its own algebraic definition, then substitute x -> x_old. Verified via direct FSA-vs-ASA comparison (bypassing FD entirely) on a 72-case corpus of affected SBML semantic test suite models: divergence for the affected parameter previously ranged from ~0.001% up to ~1% relative (worst observed: FSA=0.16456 vs ASA=0.16303); after the fix, it drops to ~1e-13-3e-11 relative (floating-point noise) with zero regressions. Corpus (SBML semantic test suite case IDs), for future reference/test cases: - Explicit SBML events (state-triggered and state-updating): 00026, 00041, 00074, 00619, 00620, 00621, 00634, 00635, 00636, 00646, 00647, 00648, 00652, 00653, 00654, 00679, 00680, 00681, 00689, 00700, 00707, 00723, 00736, 00743, 00744, 00745, 00746, 00747, 00749, 00750, 00751, 00752, 00753, 00754, 00755, 00756, 00789, 00790, 00791, 00845, 00846, 00847, 00883, 00884, 00945, 00946, 00947, 00948, 01045, 01046, 01047, 01071, 01072, 01073, 01094, 01106 - Piecewise reaction-rate laws (no explicit <event>, but AMICI hits the same code path via an internal root): 00191, 00192, 00193, 00194, 00198, 00199, 00200, 00201, 00270, 00272, 00273, 00274, 00276, 00277, 00278, 00279 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
dweindl
force-pushed
the
fix-asa-event-trigger-time-sensitivity
branch
from
September 10, 2026 16:28
33a26aa to
1aee264
Compare
dweindl
enabled auto-merge
September 10, 2026 16:29
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
deltaxB/deltaqB embed dtaudx/dtaudp (trigger-time-sensitivity terms), but these were evaluated at whatever state the generated function happens to be called with -- in practice the post-event state. That's the wrong side of the discontinuity: the equivalent forward-mode quantity (stau) is deliberately computed pre-event. Fix: eliminate w via its own algebraic definition, then substitute x -> x_old.
Verified via direct FSA-vs-ASA comparison (bypassing FD entirely) on a 72-case corpus of affected SBML semantic test suite models: divergence for the affected parameter previously ranged from ~0.001% up to ~1% relative (worst observed: FSA=0.16456 vs ASA=0.16303); after the fix, it drops to ~1e-13-3e-11 relative (floating-point noise) with zero regressions.
Corpus (SBML semantic test suite case IDs), for future reference/test cases:
Co-Authored-By: Claude Sonnet 5 noreply@anthropic.com
Fixes #3257. Still some way to go to fix #18.
Tests are implemented as part of #3251 and #3253.